fix: emit diagnostic for member access on unreduced impl type instead of panicking#9807
Conversation
|
This change is |
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
eytan-starkware
left a comment
eytan-starkware
left a comment
There was a problem hiding this comment.
@eytan-starkware reviewed 2 files and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on orizi and TomerStarkware).
crates/cairo-lang-semantic/src/expr/compute.rs line 3960 at r1 (raw file):
TypeLongId::ImplType(_) => Err(ctx.diagnostics.report( rhs_syntax.stable_ptr(db), TypeHasNoMembers { ty: long_ty.intern(ctx.db), member_name },
Consider an informative error and not just has no members.
orizi
changed the base branch from
orizi/03-31-fix_unsupported_items_in_statement_position
to
graphite-base/9807
orizi
force-pushed
the
graphite-base/9807
branch
from
5b39fa0 to
d0640ec
Compare
orizi
force-pushed
the
orizi/03-31-fix_member_access_on_unreduced_impl_type
branch
from
64b11af to
ae804b2
Compare
orizi
left a comment
orizi
left a comment
There was a problem hiding this comment.
@orizi made 1 comment and resolved 1 discussion.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on eytan-starkware and TomerStarkware).
crates/cairo-lang-semantic/src/expr/compute.rs line 3960 at r1 (raw file):
Previously, eytan-starkware wrote…
Consider an informative error and not just has no members.
will consider - but for now less interesting.
orizi
changed the base branch from
graphite-base/9807
to
orizi/03-31-fix_unsupported_items_in_statement_position
… of panicking Fixes #9788
orizi
force-pushed
the
orizi/03-31-fix_unsupported_items_in_statement_position
branch
from
d0640ec to
44fa634
Compare
orizi
force-pushed
the
orizi/03-31-fix_member_access_on_unreduced_impl_type
branch
from
ae804b2 to
9ff0d29
Compare
3 participants
Summary
Replace unreachable panic with proper error handling for member access on unreduced impl associated types. When attempting to access members on
ImplType, the code now reports aTypeHasNoMembersdiagnostic instead of panicking with an unreachable statement.Type of change
Please check one:
Why is this change needed?
The previous implementation would panic with an unreachable statement when encountering member access on unreduced impl associated types (like
H::Ain generic contexts). This caused the compiler to crash instead of providing a helpful error message to users.What was the behavior or documentation before?
When accessing members on unreduced impl associated types, the compiler would panic with the message "Impl type should've been reduced" and crash.
What is the behavior or documentation after?
The compiler now gracefully handles this case by reporting error E2057: "Type has no members" with the specific type and member name, allowing compilation to continue and providing better user experience.
Related issue or discussion (if any)
Fixes #9788
Additional context
The change also removes an unused import
cairo_lang_debug::DebugWithDbthat was only used in the removed unreachable statement. A test case has been added to verify the new error handling behavior works correctly for generic impl associated types.Note
Medium Risk
Touches core semantic/type-checking code for dot member access; while the change is small, it can affect diagnostics and control flow in the compiler frontend.
Overview
Fixes a crash in semantic expression member access: attempting
expr.memberon an unreduced impl associated type (TypeLongId::ImplType) now reportsTypeHasNoMembersinstead of hitting anunreachable!panic.Cleans up the removed debug-only import usage and adds a new
structuretest coveringH::f(x).valwhereH::Ais still unreduced, asserting errorE2057.Written by Cursor Bugbot for commit 9ff0d29. This will update automatically on new commits. Configure here.